Skip to content

Detect @Nullable on TYPE_USE parameters in configuration metadata #46854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

wonyongg
Copy link
Contributor

Update MetadataGenerationEnvironment#getAnnotation to also inspect type-use annotations (element.asType().getAnnotationMirrors()) in addition to declaration annotations. This ensures that @Nullable on method parameter types is correctly detected when generating configuration metadata for Actuator endpoints.

Previously, parameters annotated with @Nullable in TYPE_USE position could be missed, causing them to be marked as required in metadata. With this change, both declaration-level and type-use @Nullable annotations are recognized.

No new dependencies are introduced, and existing public APIs remain unchanged.

This pull request addresses the issue described in #46837.


Future improvement:
Currently, the processor only looks for org.springframework.lang.Nullable. If there is interest, we could follow up with a small, separate PR to support additional well-known @Nullable annotations (e.g. org.jspecify.annotations.Nullable, javax.annotation.Nullable).

Update MetadataGenerationEnvironment#getAnnotation to also inspect
type-use annotations (element.asType().getAnnotationMirrors()) in
addition to declaration annotations. This ensures that @nullable
on method parameter types is correctly detected when generating
configuration metadata for Actuator endpoints.

Previously, parameters annotated with @nullable in TYPE_USE
position could be missed, causing them to be marked as required
in metadata. With this change, both declaration-level and
type-use @nullable annotations are recognized.

No new dependencies are introduced, and existing public APIs
remain unchanged.

Signed-off-by: wonyongg <[email protected]>
@snicoll
Copy link
Member

snicoll commented Aug 17, 2025

Thanks for the PR.

This ensures that @nullable on method parameter types is correctly detected when generating configuration metadata for Actuator endpoints.

How do you know? Without additional tests that validates this is taken into account in the same way as OptionalParameter currently is, this PR isn't actionable.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 17, 2025
- Add NullableParameterEndpoint test sample with @nullable parameter
- Add OptionalParameterEndpoint test sample for comparison
- Add test to verify @nullable and @OptionalParameter are treated equivalently
- Ensures TYPE_USE annotation detection works correctly for optional parameters

Signed-off-by: wonyongg <[email protected]>
@wonyongg
Copy link
Contributor Author

@snicoll

Thanks for the feedback! I've added comprehensive tests that validate the @nullable annotation detection works correctly for Actuator endpoint parameters.

The tests include:

  1. A test endpoint with @nullable parameter to verify TYPE_USE annotation detection
  2. A comparison endpoint with @OptionalParameter to ensure equivalent behavior
  3. A test that verifies both annotations generate identical metadata structure

These tests demonstrate that the updated getAnnotation method correctly detects @nullable annotations on method parameters, ensuring they are treated the same as @OptionalParameter annotations when determining if a parameter is optional. This validates the core fix in the PR where TYPE_USE annotations are now properly discovered.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 17, 2025
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we're there yet. This is the wrong annotation, please review.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Aug 17, 2025
- Switch tests to org.jspecify.annotations.Nullable and remove local org.springframework.lang.Nullable
- Recognize both org.springframework.lang.Nullable and org.jspecify.annotations.Nullable as optional in the processor (backward compatible)
- Add org.jspecify:jspecify as testCompileOnly (no runtime impact)

Signed-off-by: wonyongg <[email protected]>
@wonyongg
Copy link
Contributor Author

Thanks for the review.

I’ve updated the tests to use org.jspecify.annotations.Nullable and removed the local org.springframework.lang.Nullable.
The processor now recognizes both org.springframework.lang.Nullable and org.jspecify.annotations.Nullable as optional to keep backward compatibility while aligning with JSpecify.

I also added org.jspecify:jspecify as testCompileOnly to avoid any runtime impact.

If you’d prefer a different direction (recognizing only JSpecify, keeping the processor unchanged and limiting this PR to tests, or splitting the changes), I’m happy to adjust.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants